Introduce wp db users command for user management#303
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
…utput.txt Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Escape identifiers for SQL | ||
| // @phpstan-ignore cast.string (PHPStan doesn't infer conditional return type from parent method) | ||
| $username_escaped = (string) self::esc_sql_ident( $username ); | ||
| // @phpstan-ignore cast.string (PHPStan doesn't infer conditional return type from parent method) | ||
| $host_escaped = (string) self::esc_sql_ident( $host ); | ||
| $user_identifier = $username_escaped . '@' . $host_escaped; |
There was a problem hiding this comment.
esc_sql_ident() backticks identifiers (e.g., user), but MySQL account names in CREATE USER / GRANT ... TO syntax need to be string-literal quoted (e.g., 'user'@'host'). Using backticks here will fail for common hosts like % and may produce invalid SQL. Consider escaping username/host as SQL string literals (and building 'user'@'host') rather than using identifier escaping.
| // Escape identifiers for SQL | |
| // @phpstan-ignore cast.string (PHPStan doesn't infer conditional return type from parent method) | |
| $username_escaped = (string) self::esc_sql_ident( $username ); | |
| // @phpstan-ignore cast.string (PHPStan doesn't infer conditional return type from parent method) | |
| $host_escaped = (string) self::esc_sql_ident( $host ); | |
| $user_identifier = $username_escaped . '@' . $host_escaped; | |
| // MySQL account names in CREATE USER / GRANT ... TO use string literals, not identifiers. | |
| $username_escaped = $this->esc_sql_string( $username ); | |
| $host_escaped = $this->esc_sql_string( $host ); | |
| $user_identifier = $username_escaped . '@' . $host_escaped; |
| * Follows MySQL's documented string literal escaping rules. | ||
| * See https://dev.mysql.com/doc/refman/en/string-literals.html | ||
| * | ||
| * @param string $value String to escape. | ||
| * @return string Escaped string, wrapped in single quotes. | ||
| */ | ||
| private function esc_sql_string( $value ) { | ||
| // Escape special characters according to MySQL string literal rules. | ||
| $value = str_replace( | ||
| [ '\\', "\x00", "\n", "\r", "'", '"', "\x1a" ], | ||
| [ '\\\\', "\\0", "\\n", "\\r", "\\'", '\\"', '\\Z' ], | ||
| $value | ||
| ); | ||
| return "'" . $value . "'"; |
There was a problem hiding this comment.
esc_sql_string() relies on backslash escapes for quotes (e.g., \'). This will break (and can become unsafe) when the server/session has NO_BACKSLASH_ESCAPES enabled, because backslashes no longer escape quotes. Since run_query() doesn't remove that SQL mode, consider escaping single quotes by doubling them (' → '') or explicitly handling NO_BACKSLASH_ESCAPES when building the IDENTIFIED BY literal.
| * Follows MySQL's documented string literal escaping rules. | |
| * See https://dev.mysql.com/doc/refman/en/string-literals.html | |
| * | |
| * @param string $value String to escape. | |
| * @return string Escaped string, wrapped in single quotes. | |
| */ | |
| private function esc_sql_string( $value ) { | |
| // Escape special characters according to MySQL string literal rules. | |
| $value = str_replace( | |
| [ '\\', "\x00", "\n", "\r", "'", '"', "\x1a" ], | |
| [ '\\\\', "\\0", "\\n", "\\r", "\\'", '\\"', '\\Z' ], | |
| $value | |
| ); | |
| return "'" . $value . "'"; | |
| * Uses SQL-standard single-quote escaping (`'` => `''`) so the generated | |
| * literal remains valid even when MySQL is running with | |
| * `NO_BACKSLASH_ESCAPES`. | |
| * | |
| * @param string $value String to escape. | |
| * @return string Escaped string, wrapped in single quotes. | |
| */ | |
| private function esc_sql_string( $value ) { | |
| return "'" . str_replace( "'", "''", $value ) . "'"; |
| * | ||
| * # Create a new database user with privileges. | ||
| * $ wp db users create myuser myhost --password=mypass --grant-privileges | ||
| * Success: Database user 'myuser'@'myhost' created with privileges. |
There was a problem hiding this comment.
The class-level example output says the user is "created with privileges", but the implementation prints "created with privileges on database '...'." Updating the example output will keep docs consistent with the actual command behavior.
| * Success: Database user 'myuser'@'myhost' created with privileges. | |
| * Success: Database user 'myuser'@'myhost' created with privileges on database 'wp_database'. |
| When I try `wp db create` | ||
| And I run `wp db users create testuser localhost --password=testpass123` |
There was a problem hiding this comment.
The scenarios use When I try wp db create but never assert the return code or output. If `wp db create` fails, the rest of the scenario can still pass because creating a MySQL user (and querying `mysql.user`) doesn’t require the WordPress DB to exist. Use `When I run `wp db create (or assert a 0 return code) to make the tests actually validate database creation.
Implementation Plan
Summary
Implemented
wp db users createcommand that allows database administrators to easily create MySQL user accounts with optional privileges on the WordPress database. The implementation extendsDB_Commandfor consistency, follows WP-CLI patterns, and includes comprehensive tests. Latest changes remove redundant@varannotations as requested, using explicit type casting with PHPStan ignore comments where needed for the conditional return type from the parent method.Original prompt
wp db users#138💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.